[#744] Add Farcaster wallet option to RainbowKit picker#746
[#744] Add Farcaster wallet option to RainbowKit picker#746realproject7 merged 5 commits intomainfrom
Conversation
Custom Farcaster wallet adapter wraps farcasterMiniApp() connector and appears as first option in the RainbowKit modal. Hidden outside Farcaster (iframe detection). Provides manual fallback when auto-connect fails. Auto-connect via standalone farcasterMiniApp() connector remains intact. Fixes #744 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
project7-interns
left a comment
There was a problem hiding this comment.
T2b REQUEST CHANGES — 1 issue.
Implementation is clean: custom Wallet adapter, iframe detection for hidden(), farcasterMiniApp() reuse, positioned first in modal. +12 lines, single file.
Issue: warpcast.com icon URL
iconUrl: "https://warpcast.com/favicon.ico" — project convention is to use farcaster.com URLs, not warpcast.com (Warpcast is a specific client, Farcaster is the protocol). Please use a Farcaster brand icon from farcaster.com or a local/bundled asset.
Everything else looks good.
Uses the official Farcaster brand logo as a data URI — no external dependency on warpcast.com. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The manual Farcaster wallet fallback is a good direction, but the visibility gate is too broad and will surface the Farcaster option in unrelated iframe embeds.
Findings
- [medium] The new wallet uses
window.parent !== windowas its hide/show test, which matches any iframe, not specifically a Farcaster mini app. The ticket explicitly requires the option to be shown only inside Farcaster, and the repo already has stronger Farcaster detection helpers (src/hooks/usePlatformDetection.ts,lib/farcaster-detect.ts). As written, this can expose a non-functional Farcaster wallet option anywhere PlotLink is embedded in a generic iframe.- File:
lib/wagmi.ts:25 - Suggestion: gate the wallet with the existing Farcaster SDK/context detection instead of raw iframe detection.
- File:
Decision
Requesting changes because the fallback wallet should only appear in an actual Farcaster client, and the current condition is too permissive.
project7-interns
left a comment
There was a problem hiding this comment.
T2b APPROVE — re-reviewed after fix. warpcast.com URL replaced with inline Farcaster brand SVG data URI. No external dependency. All items verified:
- Custom Wallet adapter wrapping farcasterMiniApp() ✓
- Iframe detection for hidden() ✓
- First option in Recommended group ✓
- Auto-connect unchanged ✓
- Farcaster brand icon (inline SVG, no warpcast.com) ✓
Replace generic iframe check with the same heuristic used by @farcaster/miniapp-sdk: ReactNativeWebView (mobile) or iframe embed (web). Only shows Farcaster wallet option in likely Farcaster contexts, not arbitrary iframes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The icon change is fine, but the Farcaster-only visibility gate is still not strict enough.
Findings
- [medium]
isLikelyFarcasterContext()still returns true for any iframe viaw !== w.parent. That is only the Farcaster SDK's preliminary heuristic; the SDK then verifies mini-app context before treating the environment as Farcaster. This PR skips that verification step, so the manual Farcaster wallet can still appear in non-Farcaster embeds, which does not meet issue #744's Farcaster-only requirement.- File:
lib/wagmi.ts:16 - Suggestion: gate the wallet with an actual SDK/context-backed signal, or otherwise ensure generic iframes do not surface the Farcaster option.
- File:
Decision
Requesting changes because the wallet should only appear in a real Farcaster client, and the current check can still produce false positives in generic iframe embeds.
Replace heuristic iframe check with actual Farcaster SDK context probe. Async check runs at module load and caches result — wallet defaults to hidden until SDK confirms Farcaster Mini App context. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The false-positive iframe issue is gone, but the new SDK-backed gate is now too late in the lifecycle: the manual Farcaster wallet will stay hidden even inside Farcaster.
Findings
- [high]
hidden()depends on_isFarcaster, but_isFarcasteris only flipped by an asyncimport(...).then(async ({ sdk }) => sdk.context)that runs after module evaluation.connectorsForWallets(...)builds the wallet list immediately at module load, while_isFarcasteris stillfalse, so the Farcaster wallet is filtered out before the SDK probe resolves and never gets added back.- File:
lib/wagmi.ts:20 - Suggestion: use a visibility signal that is available synchronously at connector-list construction time, or move the wallet-list construction behind a client-side flow that can wait for confirmed Farcaster context before building it.
- File:
Decision
Requesting changes because the current implementation avoids false positives by making the manual Farcaster wallet effectively unavailable everywhere, including the intended Farcaster environment.
hidden() is evaluated at connector-list construction time (not render time), so async SDK context is not viable. Switch to the same sync heuristic @farcaster/miniapp-sdk uses: ReactNativeWebView (mobile) or iframe (web). The connector itself gracefully fails outside Farcaster. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
T2b APPROVE — re-reviewed after the hidden() lifecycle discussion. Final approach is correct:
- isLikelyFarcasterContext() is a sync heuristic matching the SDK's own isInMiniApp() short-circuit
- hidden() must be sync since RainbowKit evaluates it at connector-list construction time (T2a verified this)
- Graceful failure outside Farcaster is acceptable for a manual fallback
- Inline Farcaster SVG, no warpcast.com dependency
- Well-documented with comment explaining the design constraint
Single file, clean implementation.
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The latest revision resolves the review blockers. The manual Farcaster wallet now uses the upstream miniapp SDK's own synchronous short-circuit heuristic, which is compatible with RainbowKit's hidden() lifecycle, keeps the option hidden in a normal browser, and preserves the intended manual fallback inside Farcaster.
Findings
- None.
Decision
Approving because the prior lifecycle bug is fixed and I do not see a remaining code-level mismatch against issue #744.
Summary
farcasterMiniApp()connectorwindow.parent !== window)farcasterMiniApp()connector remains intactlib/wagmi.ts(+12 lines)Fixes #744
Test plan
npm run buildpasses🤖 Generated with Claude Code